Skip to content

fix: remove runtime references from codegen [MTT-4088]#2192

Closed
ShadauxCat wants to merge 6 commits intodevelopfrom
chore/remove_runtime_references_from_codegen
Closed

fix: remove runtime references from codegen [MTT-4088]#2192
ShadauxCat wants to merge 6 commits intodevelopfrom
chore/remove_runtime_references_from_codegen

Conversation

@ShadauxCat
Copy link
Copy Markdown
Collaborator

This changes the method by which we look up types in codegen from looking it up based on the actual type to looking them up based on type names. It removes the dependencies on runtime assemblies from the codegen assembly.

This comes from the ILPP team and has a few bits of logic behind it:

  1. There's currently a bug in ILPP that's causing it to crash when running against certain platforms on certain hosts (e.g., webgl on mac) if collectibles is referenced
  2. There's a chicken and egg problem of having to load Unity.Netcode.Runtime in order to know how to modify Unity.Netcode.Runtime
  3. Most significantly, when writing to a webgl assembly, the ILPP process loads the runtime assembly, which means if there are differences between the two (such as with collections, which excludes AtomicSafetyHandle in player builds) this results in IL being generated based on the editor definition of the class instead of the player definition, which can result in IL code

The immediate benefit of this is working around the aforementioned crash, which is also being fixed by the ILPP team. This will fix it on all of our supported platforms (whereas the fix by the ILPP will only hit 2020.3, 2021.3, and 2022.x)

The long-term benefit is that this switches us to a mode of following best practices and avoids running into any other such issues in the future. The way we were doing it worked (aside from the crash) but this is the "right way" to do it.

Changelog

  • Fixed: Building for webgl on MacOS (and some other platform combinations) no longer results in a TypeLoadException

Testing and Documentation

  • No tests have been added.
  • No documentation changes or additions were necessary.

@ShadauxCat ShadauxCat requested review from a team and 0xFA11 as code owners September 9, 2022 22:22
@ShadauxCat ShadauxCat changed the title Chore/remove runtime references from codegen [MTT-4088] chore: remove runtime references from codegen [MTT-4088] Sep 9, 2022
@ShadauxCat ShadauxCat changed the title chore: remove runtime references from codegen [MTT-4088] fix: remove runtime references from codegen [MTT-4088] Sep 9, 2022
@ShadauxCat
Copy link
Copy Markdown
Collaborator Author

Guess this is more of a fix than a chore...


internal static class TypeNames
{
public const string NetworkManager = "Unity.Netcode.NetworkManager";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than hardcoding all string by hand here, we could still reference Netcode.Runtime's types but instead of typeof(XXX) type-based lookup in the ILPP/Cecil code, we could simply produce strings here with nameof(XXX) to keep it consistent with the Netcode.Runtime — also by doing that, we keep Netcode.Runtime reference around and we don't need to have Unity.Netcode.Shared asmdef and/or duplicate xxHash implementation, we could simply revert them back to current state.

public const string NetworkManager = "Unity.Netcode.NetworkManager";
public const string NetworkManager_RpcReceiveHandler = "Unity.Netcode.NetworkManager/RpcReceiveHandler";
public const string NetworkBehaviour = "Unity.Netcode.NetworkBehaviour";
public const string NetworkBehaviour___RpcExecStage = "Unity.Netcode.NetworkBehaviour/__RpcExecStage";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be the bug we're looking for (ILPP failing to make RpcExecStage protected from internal)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does adding a few string prevents the change from protected to internal ? (Not arguing it doesn't, just really curious how one leads to the other)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more specifically, this part: viour/__RpcExecSt

@0xFA11
Copy link
Copy Markdown
Contributor

0xFA11 commented Sep 13, 2022

superseded by #2199

@0xFA11 0xFA11 closed this Sep 13, 2022
@EmandM EmandM deleted the chore/remove_runtime_references_from_codegen branch October 14, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants